Fix #3569 #3572
Fix #3569 #3572
Conversation
fjtirado
force-pushed
the
Fix_#3569
branch
2 times, most recently
from
000c558 to
c285db8
Compare
There was a problem hiding this comment.
Hi @fjtirado
Thanks for the PR.
Anyway, beside other issue, I think it would be better to avoid usage of "String" for "paths".
We had a lot of issues over the last years exactly for that, and I'm afraid that this modification could bring back them, sooner or later.
Maybe, java.nio.file.Path (or similar high-level API) could be used, that automatically take care of the underlyng OS ?
Sure, Path were already being used in FileContentLoader. The change in this PR is for using String rather than URI in URIContentLoaderFactory. URIContentLoaderFactory then parse the String and create an HTTPContentLoader, FileContentLoader or ClasspathContentLoader instance. FileContentLoader, which deals with files, were already using Path (but now, rathern than obtaining the Path from the URI, it obtains it from the String) |
...orkflow-builder/src/main/java/org/kie/kogito/serverless/workflow/io/CachedContentLoader.java
Show resolved
Hide resolved
...flow-builder/src/main/java/org/kie/kogito/serverless/workflow/io/ClassPathContentLoader.java
Show resolved
Hide resolved
...low-builder/src/main/java/org/kie/kogito/serverless/workflow/io/URIContentLoaderFactory.java
Show resolved
Hide resolved
This reverts commit 24eb3c0.
As proof by this PR #3579, it seems that any change in openapi-common jar will cause the reproducible build to complain. Im afraid is because maven-invoker takes old snapshots (for indirect build only dependencies like this one). In my opinon, we can ignore the issue and merge, since once the jar is installed (as in happened in a local build) maven-invoker wont complain. |
|
@fjtirado Sorry, but I think we can't ignore reproducible failure and we have to fix it before merging this one, even if it is not strictly related. Last note: I'm afraid reproducible build is a strong requirement, and we spent lot of time to fix it, we can't simply break it again. |
|
@gitgabrio Im not saying we do not have to fix reproducible for springboot it test. Which I do not think is acceptable is to block any PR changing the openapi-common jar (and other jars which are build dependencies and not runtimes) because reproducilbe is messing up for those jars. |
|
In any case, Ill try to fix the issue on springboot it test using this PR #3579 first. |
Both disabling comparison goal for SpringBoot IT test module (see commit) or adding the openapi-common dependency for SpringBoot IT module works, but since @gitgabrio do not like the former and I do not think the latter is acceptable (because openapi-common is not a dependency of springboot it test, it is only used by Quarkus) and considering that the issue is that compare goal is including in the comparison list modules that are not SpringBoot dependencies (in the oklist in the comparison report file there is a bunch of quarkus modules) Im now trying to tell the artifact plugin that it should not consider the modified file. Its an intermediate solution that should be acceptable for anyone. |
|
thanks @fjtirado
it is not my personal choice, but how we, as community, agreed to be compliant to the apache requirements. @porcelli @baldimir : please correct me if I'm misunderstanding something.
it seems that is a transitive dependency, so, regardless, still a dependency. |
@gitgabrio There is not transitive dependency, thats the key thing. If there was, the problem wont appear because the modified jar will be installed by maven invoker in the local repository and the jars will be equal. Compare is complaining because is comparing the modified jar with the latest one available in the central repo (since the jar changed they are different) |
|
@gitgabrio |
This reverts commit 118ff4b.
…boot integration" This reverts commit c2565a9.
* [Fix apache#3569] Fixing windows path issue * [Fix apache#3569] Switching from URI to String * [Fix apache#3569] Refining for windows * [Fix_#3569] Refining uriToPath usage * [Fix apache#3569] Temporary commit to debug CI issue * Revert "[Fix apache#3569] Temporary commit to debug CI issue" This reverts commit 24eb3c0. * [Fix apache#3569] Disable reproducible comparison for spring boot integration * [Fix apache#3569] Skip module * Revert "[Fix apache#3569] Skip module" This reverts commit 118ff4b. * Revert "[Fix apache#3569] Disable reproducible comparison for spring boot integration" This reverts commit c2565a9.
Fixes #3569